Upload pending StateVersion #717
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
brandonc
force-pushed
the
brandonc/allow_state_version_create_without_state
branch
2 times, most recently
from
669a681 to
d0e9758
Compare
brandonc
force-pushed
the
brandonc/allow_state_version_create_without_state
branch
from
d0e9758 to
2b188a3
Compare
sebasslash
requested changes
Jun 14, 2023
state_version.go
Outdated
Comment on lines
276
to
284
| func (s *stateVersions) putURL(ctx context.Context, putURL string, data []byte) error { | ||
| reader := bytes.NewReader(data) | ||
| req, err := s.client.NewRequest("PUT", putURL, reader) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| return req.Do(ctx, nil) | ||
| } |
There was a problem hiding this comment.
This code is duplicated in UploadTarGzip() so I think this should belong as a Client receiver method:
// or don't export this
func (c *Client) PutArchivistURL(ctx context.Context, archivistURL string, reader io.Reader) error {
req, err := c.NewRequest("PUT", archivistURL, reader)
// error handling omitted
return req.Do(ctx, nil)
}It would eliminate the code duplication with UploadTarGzip() in RegistryModules, ConfigurationVersions and Policies
What do you think?
There was a problem hiding this comment.
OK. I don't think it should be exported but definitely shared.
There was a problem hiding this comment.
This turned into an adventure! See my notes in the PR description under "Refactor Notes"
brandonc
force-pushed
the
brandonc/allow_state_version_create_without_state
branch
2 times, most recently
from
11da9a3 to
729c2de
Compare
I added response header hook support to IPRanges. Previously, it was not possible to encounter an HTTP status error on IPRanges because this condition could never be met:
`if resp.StatusCode < 200 && resp.StatusCode >= 400 {`
brandonc
force-pushed
the
brandonc/allow_state_version_create_without_state
branch
from
729c2de to
778c6d8
Compare
sebasslash
approved these changes
Jun 15, 2023
|
Reminder to the contributor that merged this PR: if your changes have added important functionality or fixed a relevant bug, open a follow-up PR to update CHANGELOG.md with a note on your changes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Adds
pendingStateVersion support to the model as well as a new method Upload which creates and uploads and re-fetches a state version.Refactor Notes
This change alters the behavior of requests that are made directly to
IPRangesendpoint and the object store forConfigurationVersions,RegistryModels,PolicySetVersions, (and nowStateVersions). Previously, we were reusing the normal request methods for these, which unnecessarily sent an Authorization header, and additionally attempted to use jsonapi decoding (with the exception ofIPRanges, which seems to have fundamentally broken error handling)I added a new Client method,
doForeignPUTRequest, which does an object-store-style application/octet-stream request with optional JSON response handling. The "foreign" part of the name reflects the fact that we aren't making API requests that have standard API features like Authorization with tokens and rate limiting.External links
Output from tests
Including output from tests may require access to a TFE instance. Ignore this section if you have no environment to test against.